[24.2] Better cleanup of sharing roles on user purge #19096
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jdavcs
added
kind/bug
kind/enhancement
area/admin
area/database
bgruening
reviewed
Nov 1, 2024
jdavcs
force-pushed
the
dev_update_sharing_role_name
branch
from
bface56 to
ca36655
Compare
Version 24.3.dev
…entDisposition function for improved UTF-8 support
…haracters are used
apply 255 characters limit to filename exclude `/\?%*:|"<>` characters from encoded filename
Fix version: 24.3 >> 25.0
…filename Enhance UTF-8 support for filename handling in downloads
1. Update sharing role name removing purged user email 2. If sharing role has only one user association, delete it. 3. Factor out cleanup, add test. Note: this will not work if user email has been udpated after the sharing role was created.
To accommodate package unit testing
jdavcs
force-pushed
the
dev_update_sharing_role_name
branch
from
ca36655 to
df850ac
Compare
martenson
approved these changes
Nov 21, 2024
github-actions
bot
changed the title
jdavcs
changed the title
github-actions
bot
changed the title
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Builds on top of #18966 (depends on a relocated fixture from that PR, nothing more)
Currently, a user's email address is used in a sharing role's name. It's not practical to remove it because a sharing role must be identified somehow in the UI; it is possible to pull the current email for each associated user for each sharing role, but that seems like overkill, especially when multiple sharing roles need to be displayed, each potentially associated with multiple users. Using an email in the role name is for role description only: we don't retrieve roles based on email comparison anymore (fixed in #18942).
However, when a user is purged, we don't remove their email from associated sharing roles' names. This PR fixed this: we replace the user's email in the sharing role name with the string "[USER PURGED]", so a sharing role for john and alice, after john is purged, will appear as "sharing role for [USER PURGED], alice". If this sharing role has only one user association remaining with the user being purged, the role will be deleted. This also ensures there'll be no orphan sharing roles in the db (a sharing role without an associated user).
Test included.
To do in a follow-up: handle user email updates (in
api/users.py) by updating email in sharing role name.How to test the changes?
(Select all options that apply)
License